Skip to content

Conversation

@rh-jugraham
Copy link
Contributor

@rh-jugraham rh-jugraham commented Oct 14, 2025

Marked as a draft while waiting for patches that will allow these changes to be tested.

Summary by CodeRabbit

  • Tests

    • Simplified GPU hotplug test flow by removing unsupported hot‑unplug steps, focusing on hotplug validation only.
    • Reduces flaky results and false failures with NVIDIA GPUs.
  • Documentation

    • Clarified that hot‑unplug is not supported for NVIDIA GPUs.
    • Updated test logs/messages to reflect hotplug‑only behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 14, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Removed NVIDIA GPU hot-unplug/replug validation from the hotplug GPU test. The test now performs hotplug, driver validation, and final exclusivity checks only. Documentation/log text updated to reflect lack of hot-unplug support. Unused import removed.

Changes

Cohort / File(s) Summary of Changes
GPU hotplug test simplification
libvirt/tests/src/gpu/hotplug_gpu.py
Removed unplug/replug workflow and related GPU state checks; retained hotplug, driver validation, and exclusivity check. Updated logs/docs to state NVIDIA hot-unplug not supported. Cleaned imports (removed vm_xml).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test Runner
  participant L as libvirt/Host
  participant G as Guest VM
  participant D as NVIDIA Driver

  rect rgb(235, 245, 255)
  note over T,L: Current flow (hotplug-only)
  T->>L: Attach GPU (hostdev) to VM
  L-->>G: Device appears in guest
  T->>G: Validate via nvidia-smi / driver presence
  T->>L: Final exclusivity check
  end
Loading
sequenceDiagram
  autonumber
  participant T as Test Runner
  participant L as libvirt/Host
  participant G as Guest VM
  participant D as NVIDIA Driver

  rect rgb(255, 240, 240)
  note over T,L: Removed flow (unplug/replug)
  T->>L: Detach GPU (hostdev) from VM
  L-->>G: Device removed event
  T-x G: (Removed) Validate device absence
  T-x L: (Removed) Re-attach GPU and re-validate
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through logs with nimble cheer,
Hotplug stays, unplug’s not here.
GPUs snug in virtual dens,
Drivers nod with tidy grins.
Imports trimmed, the path is clean—
Thump-thump! a lean and focused scene.
Carrot commits: crisp and keen.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title directly references the primary change, namely updating the hotplug_gpu test due to unsupported hot-unplug support for NVIDIA GPUs, which matches the removal of the unplug/replug workflow in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Comment @coderabbitai help to get the list of available commands and usage tips.

@rh-jugraham rh-jugraham force-pushed the remove_hotunplug branch 3 times, most recently from 0ada0bf to 835a3e8 Compare October 14, 2025 20:02
@dzhengfy dzhengfy requested review from Yingshun and dzhengfy October 30, 2025 08:36
gpu_test.nvidia_smi_check(vm_session)

test.log.info("TEST_STEP: Check gpu device exclusivity")
res = virsh.attach_device(vm.name, gpu_dev.xml, debug=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The device will be hotplugged in Line 29, these lines can be removed I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this to align with the polarion case VIRT-304169: "Install gpu driver and run nvidia-smi in guest; nvidia-smi -q returns correct info." I did edit the case to remove hotunplug parts but this step was right before the hotunplug parts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the explanation!

@rh-jugraham rh-jugraham marked this pull request as draft October 30, 2025 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants